Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add the Context notion. #424

Merged
merged 85 commits into from
Feb 12, 2020
Merged

Add the Context notion. #424

merged 85 commits into from
Feb 12, 2020

Conversation

carlosalberto
Copy link
Contributor

This PR intends to include some of the changes of OTEP 66, by making the Specification Context aware.

Summary of changes:

  1. Add a Context mechanism section to describe what is expected in all languages overall. This is mostly described as a SDK API, in order to signal that users instrumenting their application shouldn't use it directly.
  2. Update Propagators to make them directly consume Context.
  3. Relax the Tracer's current Span handling as this now belongs to Context (changes this part from MUST to SHOULD).
  4. Removed Binary support for now. Binary needs to be updated to play along well, and it's still a work in progress anyway. The plan is to add it again in the near future, once it has been prototyped/tested.

I'm specially interested in feedback regarding 1), so feel free to provide insights ;)

@carlosalberto carlosalberto changed the title Context aware Add the Context notion. Jan 27, 2020
yurishkuro
yurishkuro reviewed Jan 27, 2020
View reviewed changes
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start.

specification/api-propagators.md Outdated Show resolved Hide resolved
specification/api-propagators.md Outdated Show resolved Hide resolved
specification/api-propagators.md Outdated Show resolved Hide resolved
specification/api-propagators.md Outdated Show resolved Hide resolved
specification/api-propagators.md Outdated Show resolved Hide resolved
specification/api-tracing.md Outdated Show resolved Hide resolved
specification/context.md Outdated Show resolved Hide resolved
specification/context.md Outdated Show resolved Hide resolved
specification/context.md Outdated Show resolved Hide resolved
specification/overview.md Show resolved Hide resolved
Oberon00
Oberon00 reviewed Jan 27, 2020
View reviewed changes
specification/api-propagators.md Outdated Show resolved Hide resolved
specification/api-propagators.md Outdated Show resolved Hide resolved
specification/api-propagators.md Outdated Show resolved Hide resolved
specification/api-propagators.md Outdated Show resolved Hide resolved
specification/api-propagators.md Outdated Show resolved Hide resolved
specification/api-propagators.md Outdated Show resolved Hide resolved
specification/api-propagators.md Outdated Show resolved Hide resolved
specification/api-propagators.md Outdated Show resolved Hide resolved
specification/api-propagators.md Outdated Show resolved Hide resolved
specification/api-tracing.md Outdated Show resolved Hide resolved
Oberon00
Oberon00 reviewed Jan 27, 2020
View reviewed changes
specification/context.md Outdated Show resolved Hide resolved
specification/context.md Outdated Show resolved Hide resolved
specification/context.md Outdated Show resolved Hide resolved
specification/context.md Outdated Show resolved Hide resolved
specification/context.md Outdated Show resolved Hide resolved
specification/context.md Outdated Show resolved Hide resolved
specification/overview.md Show resolved Hide resolved
carlosalberto and others added 4 commits January 27, 2020 14:23
@carlosalberto @Oberon00
Update specification/api-propagators.md
278839b 
Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>
@carlosalberto @Oberon00
Update specification/api-propagators.md
71f51e5 
Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>
@carlosalberto @Oberon00
Update specification/api-propagators.md
a3502c1 
Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>
@carlosalberto @Oberon00
Update specification/context.md
9be19e2 
Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>
@carlosalberto carlosalberto mentioned this pull request Feb 11, 2020
Closed
@bogdandrutu
Copy link
Member

Last comment that needs to be addressed is about Restore/Detach, once that is resolved will merge.

jkwatson
jkwatson reviewed Feb 11, 2020
View reviewed changes
specification/context.md Outdated Show resolved Hide resolved
jkwatson
jkwatson reviewed Feb 11, 2020
View reviewed changes
specification/context.md Outdated Show resolved Hide resolved
jkwatson
jkwatson reviewed Feb 11, 2020
View reviewed changes
specification/context.md Outdated Show resolved Hide resolved
dyladan
dyladan reviewed Feb 11, 2020
View reviewed changes
specification/context.md Outdated Show resolved Hide resolved
jkwatson
jkwatson reviewed Feb 11, 2020
View reviewed changes
specification/context.md Outdated Show resolved Hide resolved
jkwatson
jkwatson reviewed Feb 11, 2020
View reviewed changes
specification/context.md Outdated Show resolved Hide resolved
carlosalberto and others added 9 commits February 12, 2020 17:09
@carlosalberto @jkwatson
Update specification/api-propagators.md
a289dff 
Co-Authored-By: John Watson <jkwatson@gmail.com>
@carlosalberto @jkwatson
Update specification/api-propagators.md
27e701a 
Co-Authored-By: John Watson <jkwatson@gmail.com>
@carlosalberto @jkwatson
Update specification/context.md
980af1b 
Co-Authored-By: John Watson <jkwatson@gmail.com>
@carlosalberto @jkwatson
Update specification/context.md
c9c3cf1 
Co-Authored-By: John Watson <jkwatson@gmail.com>
@carlosalberto @jkwatson
Update specification/context.md
31af4d9 
Co-Authored-By: John Watson <jkwatson@gmail.com>
@carlosalberto @jkwatson
Update specification/context.md
a3567d9 
Co-Authored-By: John Watson <jkwatson@gmail.com>
@carlosalberto @jkwatson
Update specification/context.md
cbd3b71 
Co-Authored-By: John Watson <jkwatson@gmail.com>
@carlosalberto @jkwatson
Update specification/api-tracing.md
54735b8 
Co-Authored-By: John Watson <jkwatson@gmail.com>
@carlosalberto
Use value instead of object for Token.
349af35
@bogdandrutu bogdandrutu merged commit 33ce3e1 into open-telemetry:master Feb 12, 2020
SergeyKanzhelev pushed a commit to SergeyKanzhelev/opentelemetry-specification that referenced this pull request Feb 18, 2020
@carlosalberto @Oberon00
@MrAlias @yurishkuro @jkwatson
Add the Context notion. (open-telemetry#424)
ffc12d5 
* Initial context integration.

* Update api-propagators.md

* Remove the mention of binary propagators.

* Mention that the tracing's propagation must happen internally.

* Make context -> Context.

* Rename api-context.md to context.md

* Remove the notion of key.

* Add a note on Context being a SDK API.

* Note on the optional global operations.

* Clean up pass.

* Remove context-prop from the *tracing* index.

* Update specification/api-propagators.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/api-propagators.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/api-propagators.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/context.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* First clean up pass over Context.

* Update specification/api-propagators.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Overview and layout updates.

* Make the MD linter happy (long lines though).

* Update specification/context.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/context.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/context.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/context.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Use MUST/SHOULD for clarifying propagators expectations.

* Move the IsRemote deserialization note to the Extraction section.

* Improve wording for Context handling in Extraction.

* Clarify the protected global methods in context.md

* Remove the extra explanation of Context in overview.md

* Improve the wording for set/remove values in context.md

* Update specification/api-propagators.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/api-propagators.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/api-propagators.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/api-propagators.md

Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com>

* Fix style?

* Clarify the Context usage in Extract.

* Update specification/context.md

Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com>

* Clarify IsRemote on Extract.

* More style changes.

* OTel MUST provide Context impl if none exists.

* Update specification/context.md

Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com>

* Update specification/context.md

Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com>

* Update specification/context.md

Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com>

* Reword the optionality of Context in propagators.

* Clarify again the expected usage for implicit `Context`.

* Update specification/context.md

Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com>

* Be more strict on the Context parameters.

* Clarify that SDK/OTel instrumentation only SHOULD use global Context.

* Mention that Context will be used through concerns APIs.

* Update specification/api-propagators.md

Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com>

* Update specification/api-propagators.md

Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com>

* We do not use the Required word anymore.

* Do not require extract to set null/empty upon errors.

* Better wording for failed extraction values.

* Update specification/context.md

Co-Authored-By: Yuri Shkuro <yurishkuro@users.noreply.github.com>

* Clarify ONCE that Context is immutable.

* Use "derived" instead of "child" for Context-Context relationships.

* Remove a misleading line.

* Mention tracing uses Context specifially.

* Update specification/api-propagators.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Update specification/api-propagators.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Clarify that Context keeps the untouched original values.

* Clarify a new Context is created only for write operations.

* Misc fixes.

* Set current Context should return a handle.

* Note on global Context operations.

* Remove the Remove operation on Context.

* Clarify the usage of cross-cutting concern APIs.

* Restore "Create a key" section.

* Minor fix to a previous commit.

* Add a detach operation.

* Update specification/api-propagators.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Update specification/api-propagators.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Update specification/context.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Update specification/context.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Update specification/context.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Update specification/context.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Update specification/context.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Update specification/api-tracing.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Use value instead of object for Token.

Co-authored-by: Christian Neumüller <christian+github@neumueller.me>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Co-authored-by: John Watson <jkwatson@gmail.com>
Co-authored-by: Bogdan Drutu <lazy@splunk.com>
@codeboten codeboten mentioned this pull request Feb 25, 2020
Merged
This was referenced Feb 26, 2020
Closed
Closed
carlosalberto added a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
@carlosalberto @Oberon00
@MrAlias @yurishkuro @jkwatson
Add the Context notion. (open-telemetry#424)
a5543b9 
* Initial context integration.

* Update api-propagators.md

* Remove the mention of binary propagators.

* Mention that the tracing's propagation must happen internally.

* Make context -> Context.

* Rename api-context.md to context.md

* Remove the notion of key.

* Add a note on Context being a SDK API.

* Note on the optional global operations.

* Clean up pass.

* Remove context-prop from the *tracing* index.

* Update specification/api-propagators.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/api-propagators.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/api-propagators.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/context.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* First clean up pass over Context.

* Update specification/api-propagators.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Overview and layout updates.

* Make the MD linter happy (long lines though).

* Update specification/context.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/context.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/context.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/context.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Use MUST/SHOULD for clarifying propagators expectations.

* Move the IsRemote deserialization note to the Extraction section.

* Improve wording for Context handling in Extraction.

* Clarify the protected global methods in context.md

* Remove the extra explanation of Context in overview.md

* Improve the wording for set/remove values in context.md

* Update specification/api-propagators.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/api-propagators.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/api-propagators.md

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

* Update specification/api-propagators.md

Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com>

* Fix style?

* Clarify the Context usage in Extract.

* Update specification/context.md

Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com>

* Clarify IsRemote on Extract.

* More style changes.

* OTel MUST provide Context impl if none exists.

* Update specification/context.md

Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com>

* Update specification/context.md

Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com>

* Update specification/context.md

Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com>

* Reword the optionality of Context in propagators.

* Clarify again the expected usage for implicit `Context`.

* Update specification/context.md

Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com>

* Be more strict on the Context parameters.

* Clarify that SDK/OTel instrumentation only SHOULD use global Context.

* Mention that Context will be used through concerns APIs.

* Update specification/api-propagators.md

Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com>

* Update specification/api-propagators.md

Co-Authored-By: Tyler Yahn <MrAlias@users.noreply.github.com>

* We do not use the Required word anymore.

* Do not require extract to set null/empty upon errors.

* Better wording for failed extraction values.

* Update specification/context.md

Co-Authored-By: Yuri Shkuro <yurishkuro@users.noreply.github.com>

* Clarify ONCE that Context is immutable.

* Use "derived" instead of "child" for Context-Context relationships.

* Remove a misleading line.

* Mention tracing uses Context specifially.

* Update specification/api-propagators.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Update specification/api-propagators.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Clarify that Context keeps the untouched original values.

* Clarify a new Context is created only for write operations.

* Misc fixes.

* Set current Context should return a handle.

* Note on global Context operations.

* Remove the Remove operation on Context.

* Clarify the usage of cross-cutting concern APIs.

* Restore "Create a key" section.

* Minor fix to a previous commit.

* Add a detach operation.

* Update specification/api-propagators.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Update specification/api-propagators.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Update specification/context.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Update specification/context.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Update specification/context.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Update specification/context.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Update specification/context.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Update specification/api-tracing.md

Co-Authored-By: John Watson <jkwatson@gmail.com>

* Use value instead of object for Token.

Co-authored-by: Christian Neumüller <christian+github@neumueller.me>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Co-authored-by: John Watson <jkwatson@gmail.com>
Co-authored-by: Bogdan Drutu <lazy@splunk.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mauriciovasquezbernal mauriciovasquezbernal mauriciovasquezbernal left review comments

@dyladan dyladan dyladan left review comments

@jkwatson jkwatson jkwatson left review comments

@toumorokoshi toumorokoshi toumorokoshi left review comments

@arminru arminru arminru approved these changes

@MrAlias MrAlias MrAlias approved these changes

@jmacd jmacd jmacd approved these changes

@yurishkuro yurishkuro yurishkuro approved these changes

@MikeGoldsmith MikeGoldsmith MikeGoldsmith approved these changes

@bogdandrutu bogdandrutu bogdandrutu approved these changes

@Oberon00 Oberon00 Oberon00 approved these changes

@codeboten codeboten codeboten approved these changes

@AloisReitbauer AloisReitbauer Awaiting requested review from AloisReitbauer

@c24t c24t Awaiting requested review from c24t

@iredelmeier iredelmeier Awaiting requested review from iredelmeier

@reyang reyang Awaiting requested review from reyang

@SergeyKanzhelev SergeyKanzhelev Awaiting requested review from SergeyKanzhelev

@songy23 songy23 Awaiting requested review from songy23

@tedsuo tedsuo Awaiting requested review from tedsuo

@tigrannajaryan tigrannajaryan Awaiting requested review from tigrannajaryan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.3
Development

Successfully merging this pull request may close these issues.